Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

Thanks!

It seems like Compilation.host and Compilation.target only exist for doc tests. Perhaps those should just be removed? Then run_doc_tests can just look at the build_config to decide what to do.

Also, can you add a test for this? There are some examples in custom_target.rs. There is documentation for writing tests in CONTRIBUTING and cargo-test-support.

It seems like Compilation.host and Compilation.target only exist for doc tests. Perhaps those should just be removed? Then run_doc_tests can just look at the build_config to decide what to do.

Compilation is part of the public api though so removing host and target is technically a breaking change

Also, can you add a test for this? There are some examples in custom_target.rs. There is documentation for writing tests in CONTRIBUTING and cargo-test-support.

there is a similar bug #71002 in rustdoc that needs to get patched first. when that pr is merged I can add tests

Compilation is part of the public api though so removing host and target is technically a breaking change

Cargo's public API isn't stable, so each release is a breaking change.

It seems like Compilation.host and Compilation.target only exist for doc tests. Perhaps those should just be removed? Then run_doc_tests can just look at the build_config to decide what to do.

I played around with this and it seems to me that removing those two attribute results in messy/more complex code, so imho it's not worth removing them.

Hm, ok, I see that doctests have the unusual behavior that --target HOST still needs to work.

Perhaps add a comment explaining that, perhaps along the lines of "Use rustc_target() to properly handle JSON spec paths.".

Let me know if you have any issues adding the test.

Let me know if you have any issues adding the test.

okay so the fix in rustdoc got merged this morning, so I'm guessing it should be in the next nightly version. I'll try adding the tests tomorrow (and the comment regarding rustc_target)

the custom targets obviously can't use std, so exiting cleanly is a bit of a challenge. the only way I can think of right now is by using inline assembly and manually calling the system calls to exit. the downside of this approach is that it wouldn't be cross platform

Hm, I'm wondering, are you somehow using json specs to build std? I'm having a hard time imagining an environment where these kinds of tests would even work. (I'm only familiar with using specs with no_std.)

I'm using it for my os kernel using the bootimage/bootloader crates which procss the compiled binaries into a disk image

Can you say more of how that would work? Without std, I don't think the tests will work (maybe with a custom test runner, but those don't work with rustdoc afaik).

correct. i'm using custom test frameworks for normal tests. doctests are actually just small programs that all get compiled and executed individually.

usually rustdoc wraps the code in a main method, but if it detects that you wrote your own, it will use that instead. that way you can also use outer attributes like #![no_std] or use extern crate.

so in my case I have some hidden code in the doctests that runs the os initialization and the os uses qemu's exit decice feature to stop itself and signal whether the tests ran successfully. if i remember correctly i think i managed to get it to work at one point by manually fixing all the target path bugs.
i checked some cross compiled binaries earlier and it seems like they were normal elf files (im not sure if thats the default or if it is influenced by some config key i set). those elf files also had an entrypoint, so i was able to open them in a debugger (they pretty much instantly segfault, but thats probably caused by my kernel trying to setup memory and not because they're inherintly broken)

That is an impressive level of dedication to get doctests working! 😄

Considering that getting an actual doctest working will likely be very difficult, perhaps it should just have a test that cargo passes the correct --target flag to rustdoc. So maybe in custom_target_minimal, add something like the following:

    // Ensure that the correct style of flag is passed to --target with doc tests.
    p.cargo("test --doc --target src/../custom-target.json -v -Zdoctest-xcompile")
        .masquerade_as_nightly_cargo()
        .with_stderr_contains("[RUNNING] `rustdoc [..]--target [..]foo/custom-target.json[..]")
        .run();

with the goal just to check that it passes the correct form of --target. How does that sound?

sounds good to me

Thanks!
@bors r+

📌 Commit e5d9b03 has been approved by ehuss

⌛ Testing commit e5d9b03 with merge 6d76d7c...

☀️ Test successful - checks-azure
Approved by: ehuss
Pushing 6d76d7c to master...